feat(wallet): add ConnectWalletButton and isConnectionRestoring state#7468
feat(wallet): add ConnectWalletButton and isConnectionRestoring state#7468kernelwhisperer wants to merge 5 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
WalkthroughAdds restoring-connection detection and UI: new useIsRestoringConnection hook/export, WalletInfo flag, ConnectWalletButton component, Web3Status/Web3StatusInner prop and rendering changes, affiliate CTAs updated, and localization for "Restoring wallet...". ChangesWallet Restoring State Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying swap-dev with
|
| Latest commit: |
64b7ac2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://58d0ccb6.swap-dev-5u6.pages.dev |
| Branch Preview URL: | https://feat-wallet-restoring-spinne.swap-dev-5u6.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/modules/wallet/containers/ConnectWalletButton/index.tsx (1)
10-10: 💤 Low valueConsider exporting
ConnectWalletButtonPropsThe type is internal but since
ConnectWalletButtonis part of the public module API (re-exported fromwallet/index.ts), exporting its props type allows consumers to build typed wrappers or pass forwarded props without having to re-derive the intersection type.♻️ Proposed change
-type ConnectWalletButtonProps = ButtonPrimaryProps & { children?: ReactNode } +export type ConnectWalletButtonProps = ButtonPrimaryProps & { children?: ReactNode }And add the re-export to
wallet/index.ts:export { ConnectWalletButton } from './containers/ConnectWalletButton' +export type { ConnectWalletButtonProps } from './containers/ConnectWalletButton'As per coding guidelines, "Public module API must be re-exported via the module
index.ts" — the exported component's companion props type is part of that surface.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cowswap-frontend/src/modules/wallet/containers/ConnectWalletButton/index.tsx` at line 10, Export the ConnectWalletButtonProps type so consumers can use its shape: change the declaration of ConnectWalletButtonProps (currently type ConnectWalletButtonProps = ButtonPrimaryProps & { children?: ReactNode }) to an exported type and add a corresponding re-export from the wallet module index (so the public API exposes ConnectWalletButtonProps alongside ConnectWalletButton); update any imports that should rely on the public re-export instead of internal paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/cowswap-frontend/src/modules/wallet/containers/ConnectWalletButton/index.tsx`:
- Around line 17-24: The button currently spreads {...rest} before setting
disabled and onClick so caller-supplied disabled/onClick are ignored; fix
ConnectWalletButton by computing a final disabled value (e.g., const
finalDisabled = isConnectionRestoring || !!rest.disabled) and use
disabled={finalDisabled} on ButtonPrimary, then spread {...rest} (or spread
first and explicitly set disabled={finalDisabled} after) so a caller's
disabled=true is respected; keep the onClick override to call toggleWalletModal
when not restoring but add an inline comment above onClick in
ConnectWalletButton explaining the onClick override is intentional to avoid
future confusion.
---
Nitpick comments:
In
`@apps/cowswap-frontend/src/modules/wallet/containers/ConnectWalletButton/index.tsx`:
- Line 10: Export the ConnectWalletButtonProps type so consumers can use its
shape: change the declaration of ConnectWalletButtonProps (currently type
ConnectWalletButtonProps = ButtonPrimaryProps & { children?: ReactNode }) to an
exported type and add a corresponding re-export from the wallet module index (so
the public API exposes ConnectWalletButtonProps alongside ConnectWalletButton);
update any imports that should rely on the public re-export instead of internal
paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7e0896c4-d74f-4898-9c78-1c02c5a2b9a8
📒 Files selected for processing (10)
apps/cowswap-frontend/src/locales/en-US.poapps/cowswap-frontend/src/modules/affiliate/containers/AffiliatePartnerOnboard.tsxapps/cowswap-frontend/src/modules/affiliate/containers/AffiliateTraderOnboard.tsxapps/cowswap-frontend/src/modules/wallet/containers/ConnectWalletButton/index.tsxapps/cowswap-frontend/src/modules/wallet/containers/Web3Status/index.tsxapps/cowswap-frontend/src/modules/wallet/index.tsapps/cowswap-frontend/src/modules/wallet/pure/Web3StatusInner/index.tsxlibs/ui/src/pure/Button/index.tsxlibs/wallet/src/api/types.tslibs/wallet/src/wagmi/updater.ts
elena-zh
left a comment
elena-zh
left a comment
There was a problem hiding this comment.
hey @kernelwhisperer , the issue is partially fixed, I think.
The original issue is not 100% addressed:
- open any of affiliate pages
- connect to a wallet
- refresh the page --> the page blinks with the 'connect wallet' banner before the cards with stats are loaded.
|
@elena-zh try now please 🙏 |
|
Hey @kernelwhisperer , kinda... Might it be possible to show a generic loader on the page until the data is fetched? |
|
@elena-zh, there should be only 2 loading states left:
We do this, that's the state number 2
Could you record a video please? |
kernelwhisperer
force-pushed
the
feat/wallet-restoring-spinner
branch
from
00dab29 to
e8688c3
Compare
kernelwhisperer
force-pushed
the
feat/wallet-restoring-spinner
branch
from
e8688c3 to
831b33d
Compare
As requested, there is now just 1 loading screen, the 3 card skeleton. |
elena-zh
left a comment
elena-zh
left a comment
There was a problem hiding this comment.
Looks way better now, thank you!
4 participants
Summary
To Test
Connect walletafter restoration completesAdd codeBackground
With Wagmi/Viem we can detect wallet restoration through
useIsRestoringConnection(). The affiliate pages now treat that state like other pending page data and show the existing loading skeleton instead of briefly rendering disconnected onboarding UI.